Skip to content

fix(prefs): invalidate cached scope synchronously on setValue#9

Merged
ralflang merged 1 commit into
FRAMEWORK_6_0from
fix/setvalue-invalidates-session-cache
Jul 1, 2026
Merged

fix(prefs): invalidate cached scope synchronously on setValue#9
ralflang merged 1 commit into
FRAMEWORK_6_0from
fix/setvalue-invalidates-session-cache

Conversation

@ralflang

@ralflang ralflang commented Jul 1, 2026

Copy link
Copy Markdown
Member

Shutdown-time cache drop races SessionLifecycle::shutdown()'s mirror to $_SESSION and never reaches the persisted row. Invalidate in setValue() so the removal is part of the mirrored payload.

Depends on horde/Core PR fixing Horde_Core_Cache_Session::expire() key mismatch to actually take effect.

Refs horde/SessionHandler#13.

Dropping the cache from the shutdown-time store() path races
SessionLifecycle::shutdown()'s mirror to $_SESSION: the mirror runs
first, so the invalidation never reaches the persisted session row.
Invalidate synchronously in setValue() so the removal is part of the
mirrored payload. Refs horde/SessionHandler#13.
@ralflang ralflang requested a review from TDannhauer July 1, 2026 08:20
@ralflang ralflang merged commit d112a41 into FRAMEWORK_6_0 Jul 1, 2026
1 check failed
@TDannhauer

Copy link
Copy Markdown
Contributor

reviewed and approved, some hints:

nosave option — Cache is invalidated even when nosave: true (e.g. IMP post-login overrides). That is probably fine: in-memory updates stay in $_scopes for the current object; nosave changes were never meant to survive anyway. Worth knowing if anyone relied on stale cache surviving a nosave write in the same request across multiple Horde_Prefs instances (unlikely).

Exception type — Catches bare Exception, consistent with store() in the same class, not newer Horde_Exception style. Fine for this legacy file.

No unit test — A small test with a cache stub recording remove() calls on setValue() would lock in the contract and guard against regressing back to shutdown-only invalidation. Not required to merge given the cross-package nature of the bug.

FYI @ralflang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants